[ngyongjian] iP by ngyongjian · Pull Request #208 · nus-cs2113-AY2223S2/ip

ngyongjian · 2023-02-01T13:29:25Z

No description provided.

raydent30

Good job on your code!
Could use some reformating.

raydent30 · 2023-02-03T02:59:18Z

+                if (input.equals("list")) {
+                    output = "Here are the tasks in the list:" + System.lineSeparator() + Task.printTasksList(tasks);
+
+                } else if (input.matches("mark [0-9]{1,2}")) {
+                    String[] marks = input.split(" ");
+                    tasks.get(Integer.parseInt(marks[1]) - 1).markAsDone();
+                    output = "Nice! I've marked this task as done:" + System.lineSeparator()
+                            + tasks.get(Integer.parseInt(marks[1]) - 1).toString();
+                } else if (input.matches("unmark [0-9]{1,2}")) {
+                    String[] marks = input.split(" ");
+                    tasks.get(Integer.parseInt(marks[1]) - 1).markAsNotDone();
+                    output = "OK, I've marked this task as not done yet:" + System.lineSeparator()
+                            + tasks.get(Integer.parseInt(marks[1]) - 1).toString();
+                } else if (input.startsWith("todo", 0)) {
+                    String toDoDesc = input.split("todo")[1].trim();
+                    Task toDo = new Todo(toDoDesc);
+                    tasks.add(toDo);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + toDo.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                } else if (input.startsWith("deadline", 0)) {
+                    String deadlineDesc = input.split("/")[0].split("deadline")[1].trim();
+                    String deadlineDay = input.split("/")[1].trim();
+                    Task deadline = new Deadline(deadlineDesc, deadlineDay);
+                    tasks.add(deadline);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + deadline.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                } else if (input.startsWith("event", 0)) {
+                    String eventDesc = input.split("/")[0].split("event")[1].trim();
+                    String start = input.split("/")[1].trim();
+                    String end = input.split("/")[2].trim();
+                    Task event = new Event(eventDesc, start, end);
+                    tasks.add(event);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + event.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                } else {
+                    Task task = new Task(input);
+                    tasks.add(task);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + task.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                }


Good code readability with the else-if statements!

raydent30 · 2023-02-03T03:01:20Z

+        System.out.println("What can I do for you?\n");
+        ArrayList<Task> tasks = new ArrayList<Task>();
+        try (Scanner scan = new Scanner(System.in)) {
+            String input = scan.nextLine();


"Arrow Head" code, maybe you can consider avoiding deep nesting?

raydent30 · 2023-02-03T03:02:19Z

+                            + event.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                } else {
+                    Task task = new Task(input);


A lot of complex strings/formulas, maybe consider splitting them up?

raydent30 · 2023-02-03T03:04:43Z

+                    Task event = new Event(eventDesc, start, end);
+                    tasks.add(event);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + event.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks


A lot of "magic numbers" involved. Maybe can consider using constants instead?

Try not to use magic numbers. Can consider using an aptly named variable.

Can consider using switch case instead of if else to decide what to do based on the first word entered

YC-Michael

Overall, good naming and layout.

YC-Michael · 2023-02-03T02:43:55Z

        System.out.println("Hello from\n" + logo);
+        System.out.println("Hello! I'm Duke");
+        System.out.println("What can I do for you?\n");
+        ArrayList<Task> tasks = new ArrayList<Task>();


I like how you used a resizable array to store the tasks instead of a fixed size of 100.

YC-Michael · 2023-02-03T04:22:14Z

+        System.out.println("Hello! I'm Duke");
+        System.out.println("What can I do for you?\n");
+        ArrayList<Task> tasks = new ArrayList<Task>();
+        try (Scanner scan = new Scanner(System.in)) {


Please follow the Java coding standards when using exceptions.

Suggested change

try (Scanner scan = new Scanner(System.in)) {

try {

Scanner scan = new Scanner(System.in)

YC-Michael · 2023-02-03T04:29:59Z

+                System.out.println("___________________________________________________");
+                input = scan.nextLine();
+            }
+        } catch (NumberFormatException e) {


Good use of exceptions here

YC-Michael · 2023-02-03T04:46:30Z

@@ -0,0 +1,15 @@
+public class Event extends Task {
+    protected String start;


variables such as start and end can be more descriptive.

theopin

Good implementation on Duke so far! Some general points to improve your code:

Use of magic literals: Do try to identify such literals and replace them with named constants so that readers can better understand what your code aims to achieve
Long Main method: Due to implementation of all the main features of duke being consolidated in the main function, it seems difficult to read and understand the flow of it. Do consider refacotring these features into seperate methods to enhance understandability of the code

theopin · 2023-02-16T02:07:42Z

+                    } else if (input.startsWith("mark")) {
+                        String[] marks = input.split(" ");
+                        tasks.get(Integer.parseInt(marks[1]) - 1).markAsDone();
+                        output = "Nice! I've marked this task as done:" + System.lineSeparator()
+                                + tasks.get(Integer.parseInt(marks[1]) - 1).toString();


Try extracting such chunks into a seperate method to ease readability of the main function as it seems to be too big currently

theopin · 2023-02-16T02:09:09Z

+                        String deadlineDesc = input.split("/")[0].split("deadline")[1].trim();
+                        String deadlineDay = input.split("/")[1].trim();


Strings such as "/" and "deadline" seem to suggest usage of magic literals, do consider declaring them as named constants to enhance readability and understandability

theopin · 2023-02-16T02:12:35Z

+                    } else if (input.startsWith("event", 0)) {
+                        String eventDesc = input.split("/")[0].split("event")[1].trim();
+                        String start = input.split("/")[1].trim();


Indentation seems off here, should be 4 instead of 2

Branch level 9

add JavaDoc

ngyongjian added 8 commits January 27, 2023 10:31

Level 1. Greet, Echo, Exit

fff13bd

Level 1. Greet, Echo, Exit

d5c0add

Level 2. Add, List

900b481

Level 2

6b160da

Level 3. Mark as Done

ef4b3fb

Level 4. ToDos, Events, Deadlines

443d336

A-CodingStandard

49f3394

A-CodeQuality

03e770e

raydent30 reviewed Feb 3, 2023

View reviewed changes

YC-Michael reviewed Feb 3, 2023

View reviewed changes

ngyongjian added 5 commits February 8, 2023 00:51

A-Exception

c010346

Level 5. Handle Errors

caa5969

Organize into packages

06d3da3

Level 6

161d9d3

A-Collections

1672649

theopin reviewed Feb 16, 2023

View reviewed changes

ngyongjian and others added 14 commits February 19, 2023 11:10

Level-7

8926248

undo Level-7

2657473

branch-Level-7

a31c6c2

Level-7

9f407e0

NumberOfTask debug

cd97e4c

A-MoreOOP

a178113

Level-9

2c054d4

Merge pull request #1 from ngyongjian/branch-Level-9

713b40a

Branch level 9

add JavaDoc

10af1c0

Merge pull request #2 from ngyongjian/A-JavaDoc

8aabdd8

add JavaDoc

Update README.md

f2ad465

Update README.md

ddc4626

Update README.md

30670ce

Update README.md

478a610

ngyongjian added 2 commits March 4, 2023 15:50

Update README.md

d35fc8f

Update README.md

f8ea312

	try (Scanner scan = new Scanner(System.in)) {
	try {
	Scanner scan = new Scanner(System.in)

		@@ -0,0 +1,15 @@
		public class Event extends Task {
		protected String start;

		String deadlineDesc = input.split("/")[0].split("deadline")[1].trim();
		String deadlineDay = input.split("/")[1].trim();

Conversation

ngyongjian commented Feb 1, 2023

Uh oh!

raydent30 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YC-Michael left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theopin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants